Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update cookie security settings #1641

Merged
merged 1 commit into from
Oct 16, 2023
Merged

Update cookie security settings #1641

merged 1 commit into from
Oct 16, 2023

Conversation

anvit
Copy link
Contributor

@anvit anvit commented Jul 25, 2023

Use secure and same-site flags for atom cookies.

@anvit anvit self-assigned this Jul 25, 2023
@jraddaoui
Copy link
Contributor

Nice!

We should make sure this works with Varnish and the example configuration from: https://github.com/artefactual/atom/blob/qa/2.x/docker/etc/varnish/default.vcl

@jraddaoui
Copy link
Contributor

Also, what about the symfony cookie?

@anvit
Copy link
Contributor Author

anvit commented Oct 11, 2023

Also, what about the symfony cookie?

It's working as expected now! My VM's production config had session_cookie_secure set to false for some reason but I noticed that that's not the case for the default config. The only thing that's not updated at this point is that the symfony cookie doesn't set the same-site flag (likely because symfony 1.4 just didn't have that flag).

@anvit anvit requested a review from a team October 12, 2023 22:29
@anvit anvit added the Type: enhancement An improvement to existing functionality. label Oct 12, 2023
@anvit anvit added this to the 2.8.0 milestone Oct 12, 2023
sbreker
sbreker approved these changes Oct 13, 2023
View reviewed changes
Copy link
Member

@sbreker sbreker left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @anvit! Thanks for adding this!

I was wondering about adding httponly to atom_authenticated but I see that this cookie is accessed from AtoM's fullwidthtreeview.js so that setting would not work.

anvit added a commit that referenced this pull request Oct 16, 2023
@anvit
Fix cookie security settings (#1641)
b4e3b77 
Update atom cookies to use secure and same-site flags.
@anvit
Fix cookie security settings (#1641)
b3dd14f 
Update atom cookies to use secure and same-site flags.
@anvit anvit merged commit b3dd14f into qa/2.x Oct 16, 2023
5 of 6 checks passed
@anvit anvit deleted the dev/cookie-security-fix branch October 16, 2023 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbreker sbreker sbreker approved these changes

Assignees

@anvit anvit

Labels
Type: enhancement An improvement to existing functionality.
Projects
None yet
Milestone
2.8.0
Development

Successfully merging this pull request may close these issues.
3 participants
@anvit @jraddaoui @sbreker